security: redact private key material from SigningKey repr/str#494
Open
OscarOzaine wants to merge 2 commits into
Open
security: redact private key material from SigningKey repr/str#494OscarOzaine wants to merge 2 commits into
OscarOzaine wants to merge 2 commits into
Conversation
`__repr__` and `__str__` on signing keys previously delegated to `to_json()`, which embeds the raw CBOR hex of the private key. This caused the secret to leak in logs, exception tracebacks, debugger output, f-strings, and REPL echo. Add `_IS_SECRET = True` to `SigningKey` and `ExtendedSigningKey` (and thus all concrete payment/stake/pool subclasses). Override `__repr__` to emit a redacted placeholder containing only the class name, key type, and a non-reversible 16-hex-char fingerprint derived from the public verification key hash. `__str__` delegates to the same path. `to_json()`, `to_cbor()`, and `save()` are untouched — callers who invoke those have opted in to exporting key material. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #494 +/- ##
==========================================
+ Coverage 90.62% 90.75% +0.12%
==========================================
Files 34 37 +3
Lines 5154 5367 +213
Branches 781 808 +27
==========================================
+ Hits 4671 4871 +200
- Misses 304 308 +4
- Partials 179 188 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
|
Code looks good. There is a format issue that could be fixed with |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Calling
repr()orstr()on a signing key returned its full JSON, including the raw private key in thecborHexfield. That meant secret keys could leak into:…without the developer ever intending to export the key.
The fix
Signing keys (
SigningKey,ExtendedSigningKey, and their subclasses) now print a redacted form instead of their secret material:The
hash=value is a short, non-reversible fingerprint derived from the public verification-key hash (which is already public on-chain), so it's safe for logs while still letting you tell two keys apart.__str__now delegates to__repr__, so both are redacted.The fingerprint helper never raises — a
reprthat throws would break debuggers and logging.What does not change
Deliberate export paths are untouched:
to_json(),to_cbor(), andsave()still return/write the real key material. A warning was added toto_json()documenting that its output contains secret material. Verification keys are unaffected.Tests
New tests in
test/pycardano/test_key.pyconfirmrepr/strnever contain the private bytes, the fingerprint is stable and public-derived, and the export methods still work.